feat: Provision minimal TokenReview RBAC for OIDC auth and add SSL error logging in token parser#6240
feat: Provision minimal TokenReview RBAC for OIDC auth and add SSL error logging in token parser#6240aniketpalu wants to merge 1 commit intofeast-dev:masterfrom
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jyejare
left a comment
jyejare
left a comment
There was a problem hiding this comment.
-
The Kubernetes auth path sets a status condition (AuthorizationReadyType) to indicate RBAC provisioning success/failure. The OIDC path does not. This means operators have no visibility into whether the OIDC RBAC was actually created.
-
The existing OIDC test (featurestore_controller_oidc_auth_test.go) verifies that the Kubernetes-auth Role/RoleBinding are absent, but it does not verify that the new feast-oidc-token-review ClusterRole and per-instance ClusterRoleBinding are created with the correct rules. It also doesn't test the cleanup path (switching from OIDC to no-auth should delete the CRB).
| func (authz *FeastAuthorization) getLabels() map[string]string { | ||
| return map[string]string{ | ||
| services.NameLabelKey: authz.Handler.FeatureStore.Name, |
There was a problem hiding this comment.
getLabels() stamps the FeatureStore-specific name. But the ClusterRole feast-oidc-token-review is shared across all OIDC FeatureStore instances. The last instance to reconcile overwrites the labels with its own name. This creates misleading audit trails — the ClusterRole appears to belong to one FeatureStore when it actually serves all of them.
Recommendation: Either use instance-independent labels for the shared ClusterRole, or use an aggregated label approach.
There was a problem hiding this comment.
Addressed by using instance-independent labels for the shared ClusterRole
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
When authz: oidc is configured, the operator now provisions a dedicated feast-oidc-token-review ClusterRole and per-instance ClusterRoleBinding with tokenreviews/create permission for SA token delegation. Changes: - Add OIDC status condition (AuthorizationReadyType) for feature parity with Kubernetes auth - Use instance-independent labels for shared ClusterRole to avoid misleading audit trails when multiple FeatureStores use OIDC - Clean up Kubernetes ClusterRoleBinding when switching auth types - Add test coverage for OIDC RBAC creation and cleanup Signed-off-by: Aniket Paluskar <apaluska@redhat.com>
aniketpalu
force-pushed
the
oidc-rbac-ssl-logging
branch
from
4da984f to
15c8ec5
Compare
| @@ -335,6 +418,25 @@ func (authz *FeastAuthorization) getLabels() map[string]string { | |||
| } | |||
| } | |||
|
|
|||
| func (authz *FeastAuthorization) getSharedOidcClusterRoleLabels() map[string]string { | |||
| return map[string]string{ | |||
| services.ServiceTypeLabelKey: string(services.AuthzFeastType), | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Missing ManagedByLabelKey in getLabels() — will break informer cache filtering. All RBAC resources created by the authz module (including the new OIDC resources) will be missing app.kubernetes.io/managed-by: feast-operator. Since cmd/main.go uses this label for informer cache selectors, the operator's cache won't see these resources, causing reconciliation failures or infinite create loops.
Update it with master version.
2 participants
What this PR does / why we need it:
When `authz: oidc` is configured, the Feast server delegates Kubernetes service account (SA) tokens to a lightweight TokenReview for validation and namespace extraction. This requires the server SA to have `tokenreviews/create` permission. Previously, this RBAC was not provisioned automatically by the operator for OIDC deployments (only for `authz: kubernetes`), requiring manual ClusterRole creation.Operator: OIDC TokenReview RBAC
The operator now provisions a dedicated
feast-oidc-token-reviewClusterRole and ClusterRoleBinding whenauthz: oidcis configured. The ClusterRole contains exactly one rule:authentication.k8s.io/tokenreviews/createThis is the minimum permission needed for the SA token delegation path. No additional RBAC queries (rolebindings, clusterroles, namespaces) are granted, unlike the
authz: kubernetespath which needs broader permissions forKubernetesTokenParser.Cleanup is handled automatically when switching auth types:
SDK: SSL Error Logging
When
verify_ssl: trueis set but the OIDC provider uses self-signed certificates without a configuredca_cert_path, the server fails to reach the JWKS/discovery endpoints. Previously, this produced a generic "Invalid token" log with no indication of the root cause. The token parser now detects SSL errors in the exception chain and logs a clear, actionable message:This applies to both the discovery endpoint (
_validate_token) and the JWKS endpoint (_decode_token) error paths.Which issue(s) this PR fixes:
Follow up to #6089
Checks
git commit -s)Testing Strategy
Misc